Skip to content

Fix Conv3d docstring argument order (padding before dilation)#3793

Merged
zcbenz merged 1 commit into
ml-explore:mainfrom
Pablosinyores:fix/conv3d-docstring-arg-order
Jul 7, 2026
Merged

Fix Conv3d docstring argument order (padding before dilation)#3793
zcbenz merged 1 commit into
ml-explore:mainfrom
Pablosinyores:fix/conv3d-docstring-arg-order

Conversation

@Pablosinyores

Copy link
Copy Markdown
Contributor

The Conv3d docstring lists the constructor arguments as stride, dilation, padding, but the actual signature order is stride, padding, dilation — matching Conv1d and Conv2d.

A caller who follows the docstring order with positional arguments (e.g. nn.Conv3d(3, 16, 3, 1, 2) intending dilation=2) would instead set padding=2. Conv3d was the lone outlier here; Conv1d and Conv2d both document stride -> padding -> dilation.

Reorders the Conv3d docstring so padding precedes dilation, matching the signature and the sibling conv layers. Documentation-only, no behavior change.

@Pablosinyores

Copy link
Copy Markdown
Contributor Author

Heads-up: the red macOS (15.0) check is the flaky distributed test (mpirun -np 8 mpi_test_distributed.py segfaults, exit 139) — all 773 unit tests pass. This PR is docstring-only, so the failure is unrelated to the change.

The Conv3d docstring listed the constructor arguments as stride, dilation,
padding, but the signature order is stride, padding, dilation (matching
Conv1d and Conv2d). A caller following the docstring with positional
arguments would swap padding and dilation. Reorder the docstring to match
the signature and the Conv1d/Conv2d docstrings.
@zcbenz zcbenz force-pushed the fix/conv3d-docstring-arg-order branch from 9119d84 to 6cdcd27 Compare July 6, 2026 23:13

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@zcbenz zcbenz merged commit 12ae9ea into ml-explore:main Jul 7, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants